Skip to content

[WIP] feat(http-client-csharp): add ArgumentExpression for ref/out argument support#10845

Draft
JonathanCrd wants to merge 4 commits into
microsoft:mainfrom
JonathanCrd:joncarde/argument-expression-8399
Draft

[WIP] feat(http-client-csharp): add ArgumentExpression for ref/out argument support#10845
JonathanCrd wants to merge 4 commits into
microsoft:mainfrom
JonathanCrd:joncarde/argument-expression-8399

Conversation

@JonathanCrd
Copy link
Copy Markdown
Member

@JonathanCrd JonathanCrd commented May 30, 2026

Summary

Add a new ArgumentExpression class that wraps a ValueExpression with optional ref/out modifiers, cleanly separating argument-passing semantics from variable reference semantics.

Closes #8399

Changes

  • New ArgumentExpression class — wraps a ValueExpression with IsRef/IsOut, Write(), Accept() visitor support, and Update()
  • Simplified VariableExpression — removed IsRef/IsOut properties; now a pure variable reference
  • Simplified ParameterProvider — removed dual _asVariable/_asArgument caching pattern; added GetArgumentExpression()
  • Updated AsArgument() — now returns ValueExpression (an ArgumentExpression) instead of VariableExpression
  • Added VisitArgumentExpression to LibraryVisitor
  • Updated call sites: ClientProvider, MrwSerializationTypeDefinition, JsonPatchSnippets, ScmModelProvider
  • Added ArgumentExpressionTests with 8 tests covering ref, out, update, and ParameterProvider integration

Test Results

  • Generator: 1,533 passed
  • ClientModel: 1,355 passed
  • Spector: 15 passed

🤖 Created by JonathanCrd's copilot

@microsoft-github-policy-service microsoft-github-policy-service Bot added the emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp label May 30, 2026
@pkg-pr-new
Copy link
Copy Markdown

pkg-pr-new Bot commented May 30, 2026

Open in StackBlitz

npm i https://pkg.pr.new/@typespec/http-client-csharp@10845

commit: e7255fb

@github-actions
Copy link
Copy Markdown
Contributor

No changes needing a change description found.

@JonathanCrd JonathanCrd force-pushed the joncarde/argument-expression-8399 branch from c34da4a to 30e4276 Compare June 2, 2026 22:09
@azure-sdk-automation
Copy link
Copy Markdown

You can try these changes here

🛝 Playground 🌐 Website 🛝 VSCode Extension

JonathanCrd and others added 3 commits June 2, 2026 16:06
… support

Add a new ArgumentExpression class that wraps a ValueExpression with
optional ref/out modifiers, separating argument-passing semantics from
variable reference semantics.

Changes:
- Create ArgumentExpression to handle ref/out keywords in argument context
- Remove IsRef/IsOut from VariableExpression (now a pure variable reference)
- Simplify ParameterProvider by removing the dual _asVariable/_asArgument pattern
- Update AsArgument() to return ArgumentExpression wrapping the variable
- Update call sites: ClientProvider, MrwSerializationTypeDefinition,
  JsonPatchSnippets, ScmModelProvider
- Add VisitArgumentExpression to LibraryVisitor
- Add comprehensive ArgumentExpressionTests

Closes microsoft#8399

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Restore the legacy VariableExpression ref/out constructor and add regression tests for ArgumentExpression visitor handling.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
The pnpm setup:min step in RegenCheck CI fails on Linux runners
where NodeTool resolves Node.js 24.x to v24.14.1, which does not
satisfy the which@7.0.0 engine requirement (^24.15.0). Temporarily
relax engine-strict during workspace setup to unblock CI while the
runner images are updated.

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
@JonathanCrd JonathanCrd force-pushed the joncarde/argument-expression-8399 branch from 30e4276 to 56c20d4 Compare June 2, 2026 23:14
- Mark VariableExpression.IsRef/IsOut and legacy constructor as
  [Obsolete] to guide callers toward ArgumentExpression
- Replace ByRef() with ArgumentExpression in MrwSerializationTypeDefinition
  for method argument semantics (ref return in ScmModelProvider is correct)
- Fix dead ternary in ExpressionVisitorTests assertion
- Keep redundant 2-param VariableExpression(type, name) constructor since
  it is part of the public API surface and used widely

Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

emitter:client:csharp Issue for the C# client emitter: @typespec/http-client-csharp eng

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add better support for ArgumentExpression vs VariableExpression

1 participant